Skip to content

fix broken auto confirm policyRequirement check for org scoped reques…#7848

Merged
BTreston merged 1 commit into
rcfrom
ac/pm-39254-rc
Jun 22, 2026
Merged

fix broken auto confirm policyRequirement check for org scoped reques…#7848
BTreston merged 1 commit into
rcfrom
ac/pm-39254-rc

Conversation

@BTreston

@BTreston BTreston commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-39254

📔 Objective

📸 Screenshots

@BTreston
fix broken auto confirm policyRequirement check for org scoped request (
e513855 
#7839)

* fix broken auto confirm policyRequirement check for org scoped request

* call push command unconditionally, move checks into command

* add user role check to push notification command for auto confirm
@BTreston BTreston requested a review from a team as a code owner June 22, 2026 14:35
@BTreston BTreston requested a review from sven-bitwarden June 22, 2026 14:35
@github-actions

github-actions Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR fixes the broken auto-confirm policy check for org-scoped accept-membership requests by replacing the user-keyed AutomaticUserConfirmationPolicyRequirement lookup (which returned empty for JIT SSO users whose org record has Status=Invited, UserId=set, Email=null) with the org-scoped IPolicyQuery.RunAsync. It also consolidates the notification-gating logic into PushAutoConfirmNotificationCommand itself via three guards (org ability, policy enabled, target user role) and removes the caller-side gate in AcceptOrgUserCommand. Test coverage is comprehensive, covering each guard and the JIT SSO regression case.

Code Review Details

No findings. The DI registrations for the new IApplicationCacheService and IPolicyQuery dependencies are already in place, the new role guard is documented on the interface, and PolicyStatus.Enabled correctly defaults to false when no policy row exists.

@sonarqubecloud

sonarqubecloud Bot commented Jun 22, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@BTreston BTreston merged commit a09c7ed into rc Jun 22, 2026
40 checks passed
@BTreston BTreston deleted the ac/pm-39254-rc branch June 22, 2026 14:56
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.19%. Comparing base (6b8ed5d) to head (e513855).
⚠️ Report is 1 commits behind head on rc.

Additional details and impacted files 
@@           Coverage Diff           @@
##               rc    #7848   +/-   ##
=======================================
  Coverage   61.19%   61.19%           
=======================================
  Files        2182     2182           
  Lines       97109    97124   +15     
  Branches     8763     8765    +2     
=======================================
+ Hits        59421    59435   +14     
- Misses      35575    35576    +1     
  Partials     2113     2113

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sven-bitwarden sven-bitwarden sven-bitwarden approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BTreston @sven-bitwarden